Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Calculate wind_offshore curtailment #115

Merged
merged 3 commits into from
Jun 1, 2020
Merged

Calculate wind_offshore curtailment #115

merged 3 commits into from
Jun 1, 2020

Conversation

danielolsen
Copy link
Contributor

Purpose

Enable calculation of curtailment time series for 'wind_offshore' plants.

What is the code doing

In curtailment.py:

  • we add a reference that 'wind_offshore' profiles are obtained by calling get_wind().
  • we refactor calculate_curtailment_time_series() so that is calls get_wind() and/or get_solar() as necessary, and filters columns based on type in grid.plant so that if we are looking for just wind, or just wind_offshore, we don't get the other plants' profiles.

In test_curtailment.py: we modify tests so that they adequately test combinations of wind, solar, and wind_offshore.

Time to review

15 minutes. Besides columns selection in calculate_curtailment_time_series(), there's not much else going on.

@danielolsen danielolsen requested review from rouille and BainanXia May 27, 2020 00:01
@danielolsen danielolsen changed the title Calculate offshore curtailment Calculate wind_offshore curtailment May 27, 2020
@danielolsen danielolsen force-pushed the calculate_offshore_curtailment branch from b2c967c to 04e15ba Compare May 27, 2020 00:20
rouille
rouille reviewed May 27, 2020
View reviewed changes
postreise/analyze/generation/curtailment.py Outdated
@@ -72,7 +76,8 @@ def _check_curtailment_in_grid(curtailment, grid):
raise ValueError(err_msg)


def calculate_curtailment_time_series(scenario, resources=('solar', 'wind')):
def calculate_curtailment_time_series(
scenario, resources=('solar', 'wind', 'wind_offshore')):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PEP8 issue: the above line should be further indented

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modified to avoid this issue.

@rouille
Copy link
Collaborator

rouille commented May 27, 2020 •
edited
Loading

l. 108 in curtailment.py: should it be scenario-long instead of year-long in the docstring or something in that vein?

l. 55 and l. 174 in test_curtailment.py: ther should be 2 blank lines (PEP8)

@rouille
Copy link
Collaborator

rouille commented May 27, 2020

In the following test:

    def _check_curtailment_vs_expected(self, curtailment, expected):
        self.assertIsInstance(curtailment, dict)
        self.assertEqual(curtailment.keys(), expected.keys())
        for key in curtailment.keys():
            self.assertIsInstance(curtailment[key], pd.DataFrame)
        assert_array_equal(curtailment[key].index.to_numpy(),
                           expected[key].index.to_numpy())
        assert_array_equal(curtailment[key].columns.to_numpy(),
                           expected[key].columns.to_numpy())
        assert_array_equal(curtailment[key].to_numpy(),
                           expected[key].to_numpy())

don't we want the three assert_array_equal to be within the for loop?

@rouille
Copy link
Collaborator

rouille commented May 27, 2020

Sorry. Just realize now that I am not the reviewer. Stop here.

@danielolsen
Copy link
Contributor Author

Sorry. Just realize now that I am not the reviewer. Stop here.

Comments addressed anyway 👍

@BainanXia
Copy link
Collaborator

Sorry. Just realize now that I am not the reviewer. Stop here.

You are welcome and feel free to keep going :)

@danielolsen danielolsen force-pushed the calculate_offshore_curtailment branch from d17fea5 to c0c5485 Compare May 28, 2020 04:30
BainanXia
BainanXia approved these changes Jun 1, 2020
View reviewed changes
Copy link
Collaborator

@BainanXia BainanXia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All tests passed. Minimum changes to the existing code to support offshore wind curtailment calculations are made. This will be useful when we refactor the curtailment plot the AnalyzePG.

@danielolsen danielolsen force-pushed the calculate_offshore_curtailment branch from c0c5485 to f34573f Compare June 1, 2020 23:54
@danielolsen danielolsen merged commit 6ebc800 into develop Jun 1, 2020
@danielolsen danielolsen deleted the calculate_offshore_curtailment branch June 1, 2020 23:55
@ahurli ahurli mentioned this pull request Mar 16, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BainanXia BainanXia BainanXia approved these changes

@rouille rouille rouille left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@danielolsen @rouille @BainanXia